Skip to content

Support multiple crate versions in --extern-html-root-url #143465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kornelski
Copy link
Contributor

@kornelski kornelski commented Jul 5, 2025

Rustdoc's --extern-html-root-url used to use tcx.crate_name() to identify crates, but that used crates' internal names from their metadata, instead of names given to them in --extern. That was confusing, because both --extern… arguments seem related and use similar syntax. Crucially, this didn't work correctly with Cargo's package aliases or multiple versions of crates.

sess.opts.externs lacks CrateNum, and Resolver.extern_prelude gets destroyed before rustdoc has a chance to see it, so I've had to save this mapping in CStore.

Just in case, I've kept the previous mapping by crate name as a fallback for crates that weren't matched by their extern name.

Fixes #76296

@rustbot
Copy link
Collaborator

rustbot commented Jul 5, 2025

r? @SparrowLii

rustbot has assigned @SparrowLii.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Jul 5, 2025
@kornelski kornelski changed the title Avoid redundant lookup in CrateLoader::existing_match Support multiple crate versions in --extern-html-root-url Jul 5, 2025
@kornelski
Copy link
Contributor Author

And f9f6a70 is just a bit of redundant code I've noticed in the process, too small to give it a separate PR.

@rust-log-analyzer

This comment has been minimized.

@jyn514
Copy link
Member

jyn514 commented Jul 5, 2025

cc @aDotInTheVoid @obi1kenobi - i haven’t checked if this will help distinguish versions for the json backend too but i think it might

@kornelski
Copy link
Contributor Author

Yes, the URL is in the JSON, and can be creatively used to identify the crates.

@jyn514
Copy link
Member

jyn514 commented Jul 5, 2025

to be clear i do not think html-root-url should be used for this, i’m saying the new metadata you’ve added to CrateRoot might allow the json backend to emit structured metadata.

@Nemo157
Copy link
Member

Nemo157 commented Jul 5, 2025

#76296 doesn't demonstrate the entirety of how rustdoc has issues with resolving crate versions. There's also the case of non-renamed transitively reëxported multiple versions of a single package name.

@lolbinarycat
Copy link
Contributor

I think the name resolution method used here should be documented in the rustdoc book

@petrochenkov petrochenkov self-assigned this Jul 5, 2025
kornelski added 4 commits July 7, 2025 20:03
Tracks association between `self.sess.opts.externs` (aliases in `--extern alias=rlib`) and resolved `CrateNum`

Intended to allow Rustdoc match the aliases in `--extern-html-root-url` rust-lang#76296

Force-injected extern crates aren't included, since they're meant for the linker only
@aDotInTheVoid
Copy link
Member

CC rust-lang/cargo#8296

extern_crate.dependency_of, LOCAL_CRATE,
"this function should not be called on transitive dependencies"
);
self.set_resolved_extern_crate_name(name, cnum);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be fine to do if extern_crate.dependency_of == LOCAL_CRATE { set_resolved_extern_crate_name } instead of splitting update_extern_crate into two function.
I doubt it will cause any perf regressions, but we can run the benchmarks.

@petrochenkov
Copy link
Contributor

This is better than I originally assumed, the crate lookup algorithm is not changed, only some already calculated data is exposed.
Also, delegating --extern-html-root-url to --extern seems like the best solution that we can do here.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--extern-html-root-url uses library names to identify dependencies, failing to handle multiple versions
10 participants